[CCR] Added HLRC support for pause follow API #35216
Conversation
martijnvg
added
>non-issue
v7.0.0
:Core/Java High Level REST Client
:Distributed/CCR
martijnvg
changed the title
|
|
||
| import java.io.IOException; | ||
|
|
||
| public final class PauseFollowResponse extends AcknowledgedResponse { |
There was a problem hiding this comment.
I reused AcknowledgedResponse from rollup. If we are ok with that then maybe we should move it to another package (o.e.client.common)?
Also many APIs have exactly this same response (acknowledged field). So maybe we can change AcknowledgedResponse to be a concrete class (but still allowing subclasses to change the field name), so that for this API and others we can directly use AcknowledgedResponse class instead of introducing a subclass for each API.
There was a problem hiding this comment.
Id be perfectly happy with you moving it to the core package. Re that link and the review in it, I had suggested that exactly to the author. It looks like i forgot to have him remove abstract from the class, so feel free to do that too!
edit, suggested that exactly == we modified the code such that anyone can override the acknowledged field name, so it could be strings like submitted/deleted in the rollups code.
| @@ -786,7 +786,8 @@ public void testApiNamingConventions() throws Exception { | |||
| apiName.startsWith("graph.") == false && | |||
| apiName.startsWith("migration.") == false && | |||
| apiName.startsWith("security.") == false && | |||
| apiName.startsWith("index_lifecycle.") == false) { | |||
| apiName.startsWith("index_lifecycle.") == false && | |||
| apiName.startsWith("ccr.") == false){ | |||
so that in cases when the field name is not overwritten then there is no need for a subclass.
|
@hub-cap I've updated the PR. |
| static Request pauseFollow(PauseFollowRequest pauseFollowRequest) { | ||
| String endpoint = new RequestConverters.EndpointBuilder() | ||
| .addPathPart(pauseFollowRequest.getFollowerIndex()) | ||
| .addPathPartAsIs("_ccr/pause_follow") |
There was a problem hiding this comment.
.addPathPartAsIs("_ccr", "pause_follow")
|
|
||
| import java.io.IOException; | ||
|
|
||
| public class AcknowledgedResponseResponseTests extends AbstractXContentTestCase<AcknowledgedResponse> { |
There was a problem hiding this comment.
Hmm, im not sure we need the toXContent in AcknowledgedResponse, can we make this test look like the other tests that use AbstractXContentTestCase.xContentTester, and remove the toXContent from the AcknowledgedResponse.
Also, I think this class name has one too many Response's in its name.
There was a problem hiding this comment.
im not sure we need the toXContent in AcknowledgedResponse, can we make this test look like the other tests that use AbstractXContentTestCase.xContentTester, and remove the toXContent from the AcknowledgedResponse.
Agreed, there is no need for the response classes to serialize themselves. We just need them to be parsed from xcontent to java objects. I made this change but then realized that I then also need to change the StartRollupJobResponse, PutRollupJobResponse and StartRollupJobResponse response classes and their test. So I will do that in a follow up change if that is ok with you.
* Moved `AcknowledgedResponse` to core package * Made AcknowledgedResponse not abstract and provided a default parser, so that in cases when the field name is not overwritten then there is no need for a subclass. Relates to #33824
* master: (24 commits) Replicate index settings to followers (elastic#35089) Rename RealmConfig.globalSettings() to settings() (elastic#35330) [TEST] Cleanup FileUserPasswdStoreTests (elastic#35329) Scripting: Add back lookup vars in score script (elastic#34833) watcher: Fix integration tests to ensure correct start/stop of Watcher (elastic#35271) Remove ALL shard check in CheckShrinkReadyStep (elastic#35346) Use soft-deleted docs to resolve strategy for engine operation (elastic#35230) [ILM] Check shard and relocation status in AllocationRoutedStep (elastic#35316) Ignore date ranges containing 'now' when pre-processing a percolator query (elastic#35160) Add a frozen engine implementation (elastic#34357) Put a fake allocation id on allocate stale primary command (elastic#34140) [CCR] Enforce auto follow pattern name restrictions (elastic#35197) [ILM] rolling upgrade tests (elastic#35328) [ML] Add Missing data checking class (elastic#35310) Apply `ignore_throttled` also to concrete indices (elastic#35335) Make version field names more meaningful (elastic#35334) [CCR] Added HLRC support for pause follow API (elastic#35216) [Docs] Improve Convert Processor description (elastic#35280) [Painless] Removes extraneous compile method (elastic#35323) [CCR] Fail with a better error if leader index is red (elastic#35298) ...
* Moved `AcknowledgedResponse` to core package * Made AcknowledgedResponse not abstract and provided a default parser, so that in cases when the field name is not overwritten then there is no need for a subclass. Relates to elastic#33824
jpountz
removed
the
:Distributed/CCR
This change also includes the docs for hlrc pause follow API.
I picked a simple api to get the hlrc work started for ccr.
Relates to #33824
/cc @hub-cap